Skip to content

Fix P0.1#2

Merged
jmecom merged 4 commits into
mainfrom
jm/working-on-todo
Dec 25, 2025
Merged

Fix P0.1#2
jmecom merged 4 commits into
mainfrom
jm/working-on-todo

Conversation

@jmecom
Copy link
Copy Markdown
Owner

@jmecom jmecom commented Dec 25, 2025

No description provided.

Jordan Mecom and others added 4 commits December 24, 2025 22:14
- Add ResultKind::Unit for match expressions returning unit
- Fix ResultOut handling to use ValueRepr::Unit instead of iconst(0)
- Allow match arms to return Unit (zero values, no stack slots)
- Allow matching on Unit values using a dummy comparison value
- Fix ResultOut/ResultString placeholders to use ValueRepr::Unit
- Fix implicit unit return to only emit when block doesn't end with return
- Add tests: unit_return.cap and unit_match_arm.cap

This ensures unit is consistently zero-sized across the codegen,
fixing ABI mismatches where unit was incorrectly represented as i32.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- value_from_params now errors on ResultOut/ResultString params
  These are ABI return types and should never appear as input params.
  Returning an error prevents silently hiding bugs.

- Add pattern validation for match-on-unit
  When matching on Unit values, validate that only wildcard (_),
  binding, or unit literal patterns are used. This prevents generating
  nonsense comparisons against the dummy match value.

- Add tests:
  - unit_match_multi.cap: Match expression with Unit-returning arms
  - result_unit_ok.cap: Result<Unit, i32> payload handling

All tests pass (21 run tests, 29 typecheck tests).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Change unit match dummy from I8 to I32 to align with enum dispatch
  This reduces risk of accidental type mismatches since enum variants
  (the most common match case) use I32 tags.

- Add support for single-segment Path patterns as bindings
  The parser currently emits Pattern::Path for bare identifiers in
  match arms (e.g., `match () { x => ... }`). Added handling in:
  - match_pattern_cond: single-segment paths always match (like bindings)
  - bind_match_pattern_value: bind the value to the path identifier
  - Unit match validation: allow single-segment paths

- Verified that store_local correctly handles Unit without creating
  dummy values - it stores LocalValue::Value(ValueRepr::Unit) with
  no stack allocation, so bindings on unit are safe.

- Add unit_match_bind.cap test to verify binding patterns work
  correctly with unit values without forcing dummy value storage.

All 22 run tests and 29 typecheck tests pass.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
A) Improve always-true pattern condition clarity
   Changed from `icmp_imm(NotEqual, iconst(I8, 1), 0)` to:
   `let one = iconst(I32, 1); icmp_imm(Equal, one, 1)`

   While both work, the new version makes intent clearer (1 == 1 is
   obviously true). Attempted to use bconst(B1, true) but it doesn't
   exist in this Cranelift version. The explicit comparison is more
   readable than "1 != 0" and uses I32 (matching most other constants).

B) Remove Result binding restriction
   Previously bind_match_pattern_value rejected ValueRepr::Result.
   This was overly restrictive since store_local/load_local handle
   Result values correctly:
   - store_local: Result -> LocalValue::Value(Result)
   - load_local: LocalValue::Value(v) -> v.clone()

   Now binding patterns can bind any ValueRepr type, including Result.
   This enables patterns like `match result_value { x => ... }` where
   x is bound to the entire Result (though the parser doesn't currently
   use this pattern in practice - Ok/Err destructuring is more common).

All 22 run tests and 29 typecheck tests pass.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@jmecom jmecom merged commit 3844274 into main Dec 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant